Skip to content

Add identity to TerminatedFailureInfo + CanceledFailureInfo#745

Open
stephanos wants to merge 2 commits intomasterfrom
stephanos/termination-identity
Open

Add identity to TerminatedFailureInfo + CanceledFailureInfo#745
stephanos wants to merge 2 commits intomasterfrom
stephanos/termination-identity

Conversation

@stephanos
Copy link
Copy Markdown
Contributor

@stephanos stephanos commented Mar 27, 2026 •
edited
Loading

What changed?

Add identity field to TerminatedFailureInfo and CanceledFailureInfo.

Why?

To allow storing the identity of the caller that requested the cancellation/termination; and allow clients to retrieve it.

Breaking changes

No

Server PR

NA - usage example:

&nexusoperationpb.OperationOutcome_Failed{
  Failure: &failurepb.Failure{
    Message: event.Reason,
	FailureInfo: &failurepb.Failure_TerminatedFailureInfo{
	  TerminatedFailureInfo: &failurepb.TerminatedFailureInfo{
        identity: event.Identity,
      },
    },
  },
}

@stephanos stephanos force-pushed the stephanos/termination-identity branch from d0c7dbd to d7d9fce Compare March 27, 2026 18:15
@stephanos stephanos marked this pull request as ready for review March 27, 2026 20:20
@stephanos stephanos requested review from a team March 27, 2026 20:20
Quinn-With-Two-Ns
Quinn-With-Two-Ns reviewed Mar 30, 2026
View reviewed changes
temporal/api/failure/v1/message.proto

message TerminatedFailureInfo {
// The identity of the worker or client that requested the termination.
string identity = 1;
Copy link
Copy Markdown
Contributor

@Quinn-With-Two-Ns Quinn-With-Two-Ns Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does CanceledFailureInfo not need the same field? Like if we need the identity for terminate why not cancel?

Copy link
Copy Markdown
Contributor Author

@stephanos stephanos Mar 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me to add it there, too. The only difference AFAICT is that it needs to be threaded through there from the original "cancel" request.

Copy link
Copy Markdown
Contributor

@Quinn-With-Two-Ns Quinn-With-Two-Ns Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is that different then terminate? Isn't it also threaded through?

Copy link
Copy Markdown
Contributor Author

@stephanos stephanos Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT for "terminate" the TerminatedFailureInfo will be created in the same request as the termination request; but for "cancel" - at least for Nexus - first we need to store the "cancel request" (with the identity) and then once the cancellation actually completes can we create CanceledFailureInfo (and pull the identity info through). No big deal, and maybe this can be improved/simplified, but I just thought I'd mention it.

@stephanos stephanos changed the title Add identity to TerminatedFailureInfo Add identity to TerminatedFailureInfo + CanceledFailureInfo Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Quinn-With-Two-Ns Quinn-With-Two-Ns Quinn-With-Two-Ns left review comments

@Sushisource Sushisource Sushisource approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@stephanos @Sushisource @Quinn-With-Two-Ns